Update shared memory proposal#4620
Conversation
|
@aam @lrhn @mkustermann please take a look at the APIs. Among others, I would like to cover the following use case: Pointer<Void> threadBody(Pointer<Void> ptr) {
final isolate = Isolate.fork();
isolate.runSync(() {
// Do something on the current thread.
});
// Spin event loop on this thread until nothing left to do
isolate.runEventLoopSync();
isolate.shutdown();
}
pthread_create(..., NativeCallable.isolateGroupBound(threadBody)); |
lrhn
left a comment
lrhn
left a comment
There was a problem hiding this comment.
I'm being pedantic because I've seen too many cases of "everybody knows what this obviously means" turning into everybody thinking it obviously means different things.
So use words and phrases consistently, internally and with existing documentation.
Don't use a new word if an existing word is already in use.
If a new concept is important, define it and give it a unique name. (And define the concepts at the user/API level, nobody else cares about the implementation.)
|
|
||
| /// Create a new isolate in the current isolate group. | ||
| /// | ||
| /// Similar to `Dart_CreateIsolateInGroup` Dart VM C API. |
There was a problem hiding this comment.
Is this line useful to a reader? (Should I go read the docs on that function. If so, link!)
If not, I don't see the relevance for a Dart user reading this API.
(If it actualy calls that function, it might be relevant information, at some abstraction level. If it's just sort-of similar to a function that I don't know and can't be bothered to look at, that's just noise.)
There was a problem hiding this comment.
I wish I could link to these - but we don't publish documentation for these. So the best I could do is to link to sources, which is a bit wonky.
It is hard to say whether these are useful or not - if reader is familiar with C API layer they are certainly useful because most of APIs introduced here are exposing functionality which is available through C API.
I would prefer to keep these here for the purpose of spec review / but we can strip them later when landing actual code.
| /// executing code in the target isolate. | ||
| /// | ||
| /// Throws [TimeoutException] if [timeout] has been reached while waiting | ||
| /// to acquire exclusive access to the isolate. |
There was a problem hiding this comment.
All these thread-based members of isolate, would it make sense to put them somewhere else than in Isolate, like in dart:isolate_threads, where they could be extension members on Isolate if we really want them there.
Do we expect most people who use isolates to also want to use these thread-based functions, or will they clutter the API for the "normal" uses of Isolate and enocurage people to use them who really shoudn't?
There was a problem hiding this comment.
Yeah, these are fairly low-level. Maybe we need to introduce dart:threading?
| /// thus can't be entered from a different thread. See | ||
| /// [markOwnedByCurrentThread] and [isOwnedByCurrentThread]. | ||
| /// | ||
| /// Throws [ArgumentError] if the target isolate belongs to another |
There was a problem hiding this comment.
How can an isolate obtain an Isolate object from a different group?
There was a problem hiding this comment.
spawnUri gives you an Isolate object in a different group.
| /// between turns of its event loop, when no other thread is | ||
| /// executing code in the target isolate. | ||
| /// | ||
| /// Throws [TimeoutException] if [timeout] has been reached while waiting |
There was a problem hiding this comment.
If an isolate is dead but someone uses the Isolate object, then what happens? Will an operation timeout will it throw an error?
I guess Isolate is a weak reference to an actual isolate. So using it has to turn the weak reference into a strong reference using some global data structure with locking (bit like PortMap).
There was a problem hiding this comment.
If isolate dies operations with it should throw an error - Isolate is just a port number and yeah all operations with it are based on that. We can guard against dead isolates to some extent - though if port number gets reused you will get strange effects. But I am not sure this can be fully resolved.
| /// * integrate isolate's event loop with other event loop by registering | ||
| /// message callback ([Isolate.onMessage]) and draining pending messages | ||
| /// ([Isolate.handleMessage]). | ||
| external static Isolate fork({String? debugName}); |
There was a problem hiding this comment.
I find the name fork strange as a C fork() call actually gives you a copy of the state of the process, there's no clearing of state to some initial state happening.
Furthermore from the motivating example in #4620 (comment) it seems that the caller may not even be an isolate with state. So when a Thread (aka shared isolate) calls Isolate.fork() it gets an isolate with freshly initialized state.
Why not call it Isolate.create() instead?
There was a problem hiding this comment.
Renamed to Isolate.create.
In addition to If we ever wanted to for example reimplement the embedder in Dart, we'd need this for resize synchronization (i.e. we want to be able run the dart event loop in a blocking way until surface with appropriate size is generated). Right now we have an implementation for this per platform (a custom CFRunLoopMode on macOS or polling the loop on windows). This only processes Flutter messages and ignores the platform events. |
|
@knopp polling is implementable on top of |
|
Not sure what's the good way to submit feedback to the proposal, just something I've noticed:
Just a nit, but after dart-lang/sdk#61623, |
|
@lrhn @mkustermann I think I addressed most of comments. I think the main question that remains is whether or not to move low-level isolate members to a separate library. I am starting to think that maybe |
|
I am going to land this and @aam is working to implementing the prototype. We have few remaining unanswered questions (e.g. where should new methods reside), but we can address them later. |
5 participants
finalvariables and fields can't be late when describing deeply immutable types.Isolate.